Update filter in filtered reply times CTE in int_zendesk__reply_time_combined #134
Conversation
There was a problem hiding this comment.
@fivetran-reneeli great job on working on this tricky bugfix! I do have a few questions included in my review that I would like you to take a look at before this will be ready to be approved. Let me know if you have any questions!
models/sla_policy/reply_time/int_zendesk__reply_time_combined.sql
Outdated
Show resolved
Hide resolved
models/sla_policy/reply_time/int_zendesk__reply_time_combined.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
@fivetran-reneeli thanks again for working through this PR. This is looking really good, I just have a few final questions and suggestions before this should be good to approve.
Let me know if you want to chat about any of these final comments in more detail.
models/sla_policy/reply_time/int_zendesk__reply_time_combined.sql
Outdated
Show resolved
Hide resolved
models/sla_policy/reply_time/int_zendesk__reply_time_combined.sql
Outdated
Show resolved
Hide resolved
Co-authored-by: Joe Markiewicz <74217849+fivetran-joemarkiewicz@users.noreply.github.com>
There was a problem hiding this comment.
Thanks for applying these changes @fivetran-reneeli! This looks good to go from my end. Be sure to regen the docs and then this is ready for release review.
|
Consolidating this into PR #136 |
PR Overview
This PR will address the following Issue/Feature:
#131
It was discovered that some tickets were not persisting through the final
sla policiesmodel, and mistakenly filtered out at theint_zendesk__reply_time_combinedmodel.This PR will result in the following new package version:
v0.13.2
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
Bug Fixes
int_zendesk__reply_time_combinedmodel to account for when a ticket is first replied to outside SLA schedules or not yet replied to.Under the Hood
filtered_reply_timesCTE for business hours to account for the aforementioned cases, as these records were mistakenly being filtered out previously.PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
The appropriate issue has been linked, tagged, and properly assigned
All necessary documentation and version upgrades have been applied
docs were regenerated (unless this PR does not include any code or yml updates)
BuildKite integration tests are passing
Detailed validation steps have been provided below
tested with internal data, validations linked in height ticket
Detailed Validation
Please share any and all of your validation steps:
int_zendesk__reply_time_combinedmodel, and confirmed they are in fact expected to exist in the end models and not be filtered out. Then adjusted the filters in thefiltered reply timesCTE, ran the package, and confirmed that the tickets persisted in the endsla policiesmodel.If you had to summarize this PR in an emoji, which would it be?
📆